[VL] Deserialize broadcast build side per the consuming stage's cuDF tag - #12838
Conversation
…uDF conf BuildSideRelation#deserialized creates its Runtime from the session conf, so with spark.gluten.sql.columnar.cudf=true VeloxRuntime::createColumnarBatchSerializer returns the GPU serializer and every broadcast batch is uploaded to the device, even when the consuming stage was not offloaded to cuDF and planned a host-contract value stream. TPC-H q16 reproduces this deterministically: its not-in subquery is a null-aware anti join, which Spark always executes as a broadcast join even with spark.sql.autoBroadcastJoinThreshold=-1, so the untagged consumer receives device-resident CudfVectors and fails. Pass a per-instance COLUMNAR_CUDF_ENABLED=false override so broadcast batches are always deserialized to host. Both serializer flavors share the same wire format (the GPU one only overrides deserialize), and cuDF consumers upload host batches themselves via CudfVectorStream.
|
The join stage of BHJ is supported on GPU. We shouldn't set |
marin-ma
left a comment
There was a problem hiding this comment.
Based on my understanding, the gap lies in the VeloxGpuColumnarBatchSerializer is always called regardless of the hash join stage is executed on cpu or gpu. When cudf is enabled, the query fails when the hash join is executed on CPU, but can pass on GPU.
This change will cause the hash join to fail when it is executed on the GPU.
|
Tested tpcds q95 locally with a small dataset. Before this change it can pass, but with this change it fails. |
I agree with you on this. The missing piece is #12471, which uploads in Also on autoBroadcastJoinThreshold=-1; I only used it to isolate q16 to a single broadcast, and I'll re run at the default. So there are two options we could do:
I lean toward 2. Happy to do either |
|
I wouldn't agree with the changes in #12471 Cudf related execution shouldn't be introduced into Please continue with solution 2. Based on the current support status, after the fix the correct pipeline for BHJ should be: GPU stage: CPU stage: |
|
@marin-ma Pushed, the deserializer now follows the consuming stage's offloadCuda Traced on q95: One flag: dropping #12471 exposes q3/q10/q18, failing at |
|
The change looks good. Can you help to add a unit test? Thanks. |
The idea of #12471 is slightly off track. If the failures in q3/q10/q18 are unrelated to BHJ, I’d suggest closing #12471 and opening a new issue/pr. |
|
Thanks for the review @marin-ma |
| } | ||
|
|
||
| test("GLUTEN-12838: broadcast build side follows the consuming stage's cuDF tag") { | ||
| // We need a broadcast that lands in a CPU stage. NOT IN gives us one for free: it |
There was a problem hiding this comment.
@ReemaAlzaid Thanks for adding the test. My understanding is that we need to test broadcast in a CPU stage + hash join in a CPU stage when cudf is enabled based on your issue description:
VeloxRuntime::createColumnarBatchSerializer returns VeloxGpuColumnarBatchSerializer and every broadcast batch is uploaded to the device even when the consuming stage was not offloaded to cuDF and planned a host-contract value stream.
Does this test cover this case?
There was a problem hiding this comment.
Yes. I instrumented the test to check the tag directly:
bhj offloadCuda=false nullAware=true
stage offloadCuda=false (both WholeStageTransformers in the plan)
So both the broadcast hash join and its stages are not offloaded, even with spark.gluten.sql.columnar.cudf=true. This is exactly the case where the session config was incorrectly selecting the GPU serializer and passing device-resident CudfVectors into a host side value stream.
I also confirmed the test catches the regression. Reverting the serializer selection back to the session config makes it fail with:
childAt: index < childrenSize_ (0 vs. 0)
on:
ROW<"node_value-stream:0_0":BIGINT, …>
because the CPU stage receives a device vector with no children.
With the fix, both tests pass.
There was a problem hiding this comment.
Can you the tag check for BHJ to the test?
There was a problem hiding this comment.
Added. The test now asserts the BHJ is not offloaded to cuDF
marin-ma
left a comment
There was a problem hiding this comment.
LGTM. Thanks for iterating on this!
What changes are proposed in this pull request?
BuildSideRelation#deserializedbuilt its Runtime from the session conf, sobroadcast batch residency was decided session wide, while the consuming
stage's value stream contract is decided per stage by the cuDF tag. The two
disagree in both directions: an untagged stage receives device-resident
batches on a host contract
ValueStreamNode(TPC-H q16), and a tagged stage'sCudfHashJoinBuildrequires device input (TPC-DS q95).Pass the consuming stage's
offloadCudainto deserialized instead, so acuDF offloaded stage gets the GPU serializer and a non-offloaded stage gets
the host one. This mirrors the shuffle read path, which already selects its
deserializer from the consumer's output type rather than the session conf.
BroadcastUtilskeeps the no arg deserialized, which is now host-resident —it feeds
VeloxColumnarToRowand wants host batches in every mode.How was this patch tested?
Tested on an L40S without #12471 applied: TPC-DS q95 and TPC-H q16 both pass
in hybrid and pure-GPU.
Was this patch authored or co-authored using generative AI tooling?